Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce number of log IDs allocated to vehicles, rearrange #19562

Merged
merged 8 commits into from
Dec 20, 2021

Conversation

peterbarker
Copy link
Contributor

@peterbarker peterbarker commented Dec 18, 2021

With the following patch:

--- a/ArduPlane/ArduPlane.cpp
+++ b/ArduPlane/ArduPlane.cpp
@@ -286,6 +286,9 @@ extern AP_IOMCU iomcu;
 
 void Plane::one_second_loop()
 {
+
+    gcs().send_text(MAV_SEVERITY_INFO, "SCR=%u MODE=%u CMDH=%u", LOG_SCRIPTING_MSG, LOG_MODE_MSG, LOG_CMDH_MSG);
+
     // make it possible to change control channel ordering at runtime
     set_control_channels();

I get the following output:

AP: SCR=197 MODE=112 CMDH=16

Which means we have 57 ids "after the break" - available for logger.Log_write(...) and logger.LogWriteStreaming(...) to use. We don't have that many calls - and if anyone's using more in scripting then they're almost certainly doing something very wrong...

This also introduces a gap before FMT - before this patch MODE was 127. That's very silly as anybody adding a logging message to e.g. NavEKF3 would get a compilation failure as it would push MODE past FMT!

Also adds a comment to warn people of the 32-id limit. Plane's the largest user of vehicle-specific IDs with 17 defined.

Also corrected a bad sanity check.

Tidied the vehicle definitions up to remove absolute setting of specific IDs (not needed), and to make enumerations in the two vehicles where we were using defines.

Tested only with the autotest suite and a very small amount in SITL.

Edit: I flashed Rover onto CubeBlack and had a look at a produced log. All looked in order.

Add a comment indicating only 20 ids are available to the vehicle
Add a comment indicating only 20 ids are available to the vehicle
Add a comment indicating only 20 ids are available to the vehicle
Add a comment indicating only 20 ids are available to the vehicle
Add a comment indicating only 20 ids are available to the vehicle
Add a comment indicating only 20 ids are available to the vehicle
ArduPlane uses ~18 messages and the list is relatively static, so this
should be a reasonable reallocation.

We're using a lot of Log_Write(...) to create messages dynamically - but
that requires IDs and we only left space for about 18 before this patch

More space is left to ease future use of IDs in some libraries
@tridge tridge merged commit 600b085 into ArduPilot:master Dec 20, 2021
@peterbarker peterbarker deleted the pr/move-ids-around branch December 21, 2021 10:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants